feat(snack bar): Add enter and exit animations. #1320

josephperrott · 2016-09-24T00:12:18Z

Continuing #115

jelbourn

For testing, I'm thinking it would make sense to test that the state changes are happening but to not make any assertions about the animations tied to those states.

jelbourn · 2016-09-25T00:33:20Z

src/lib/snack-bar/snack-bar-container.scss

  min-width: $md-snack-bar-min-width;
  overflow: hidden;
  padding: $md-snack-bar-padding;
+  transform: translateY(100%);


Add comment explaining what the initial transform is for.

jelbourn · 2016-09-25T00:34:39Z

src/lib/snack-bar/snack-bar-container.ts

+import {Subject} from 'rxjs/Subject';
+
+
+export class SnackBarState {


Can we do this with a string literal type? E.g.,

type SnackBarState = 'initial' | 'visibile' | 'complete' | 'void';

I think this would make the animation definitions much cleaner.

Sure thing, I was thinking that this way allowed us to have a place to hold the values centrally, rather than strings in different places.

jelbourn · 2016-09-25T00:38:28Z

src/lib/snack-bar/snack-bar-container.ts

+      state(SnackBarState.visible, style({transform: 'translateY(0%)'})),
+      state(SnackBarState.complete, style({transform: 'translateY(100%)'})),
+      transition(`${SnackBarState.visible} => ${SnackBarState.complete}`,
+                 animate('250ms cubic-bezier(0.4, 0.0, 1, 1)')),


If these are the universal curves for entry and exist, can we drop them into constants in something like core/animation?

Done, added both duration and easing curves.

jelbourn · 2016-09-25T00:45:32Z

src/lib/snack-bar/snack-bar-container.ts

  @ViewChild(PortalHostDirective) _portalHost: PortalHostDirective;

+  /** Subject for notifying that the snack bar has removed from view. */
+  private _state: Subject<any> = new Subject();


_whenDestroyed?

Or _onExit would also work.

jelbourn · 2016-09-25T00:48:02Z

src/lib/snack-bar/snack-bar-container.ts

+  private _state: Subject<any> = new Subject();
+
+  /** The state of the snack bar animations. */
+  state: string = SnackBarState.initial;


Can we call this animationState?

jelbourn · 2016-09-25T00:49:24Z

src/lib/snack-bar/snack-bar-container.ts

+  }
+
+  /** Mark snack bar as exited from the view. */
+  markDestroyed(event: AnimationTransitionEvent) {


markAsDestroyed ?

called markAsExited

jelbourn · 2016-09-25T00:55:21Z

src/lib/snack-bar/snack-bar.ts

    let snackBarContainer = this._attachSnackBarContainer(overlayRef, config);
    let mdSnackBarRef = this._attachSnackbarContent(component, snackBarContainer, overlayRef);
+
+    if (this._snackBarRef) {


Add comment like

// If there is already a snackbar open, dismiss it and open a new one // only after the close animation is complete.

jelbourn · 2016-09-25T00:56:18Z

src/lib/snack-bar/snack-bar-container.ts

  }
+
+  /** Begin animation of the snack bar exiting from view. */
+  destroy(): Observable<void> {


Call this exit for symmetry with enter?

Yeah, I initially was looking for this to actually destroy itself, but ultimately just ended up with the exit.

jelbourn · 2016-09-26T11:31:25Z

src/lib/core/animation/duration.ts

+  Complex: '375ms',
+  Entering: '225ms',
+  Exiting: '195ms',
+};


Can you change these constants to the static class format (how you had the enum for animation state earlier), e.g.,

export class AnimationDuration { static Complex = '375ms'; ... }

Also, I think we can combine this file and the next into just animation.ts (it's fine that it's animation/animation.ts)

jelbourn · 2016-09-26T11:35:43Z

src/lib/snack-bar/snack-bar-container.scss

  min-width: $md-snack-bar-min-width;
  overflow: hidden;
  padding: $md-snack-bar-padding;
+  /** Initial transformation is applied to start snack bar out of view. */


Can you use the // style comments? I recently changed all of the scss comments to // when I did the theming.

jelbourn · 2016-09-26T11:36:32Z

src/lib/snack-bar/snack-bar-container.ts

  snackBarConfig: MdSnackBarConfig;

-  /** Attach a portal as content to this snack bar container. */
+  constructor(private ngZone: NgZone) {


jelbourn · 2016-09-26T12:00:59Z

src/lib/snack-bar/snack-bar-container.ts

+  markAsExited(event: AnimationTransitionEvent) {
+    if (event.fromState === 'visible' &&
+        (event.toState === 'void' || event.toState === 'complete')) {
+      this.ngZone.run(() => this._state.complete());


You should call next() in addition to complete(), that was there's still an "event" in the stream and not just the completion of the stream. This lets you subscribe like

container.exit().subscribe(() => { ... });

jelbourn · 2016-09-26T12:10:26Z

src/lib/snack-bar/snack-bar-container.ts

  @ViewChild(PortalHostDirective) _portalHost: PortalHostDirective;

+  /** Subject for notifying that the snack bar has removed from view. */
+  private _state: Subject<any> = new Subject();


Or _onExit would also work.

jelbourn · 2016-09-26T12:27:19Z

src/lib/snack-bar/snack-bar-ref.ts

-      this._afterClosed.complete();
+      this.containerInstance.exit().subscribe(null, null, () => {
+        this._overlayRef.dispose();
+        this._afterClosed.complete();


next() in addition to complete()

jelbourn · 2016-09-26T12:43:00Z

src/lib/snack-bar/snack-bar.ts

-
-    this._snackBarRef = snackBarRef;
-    return snackBarRef;
+    return <MdSnackBarRef<T>> new MdSnackBarRef(contentRef.instance,


I want to say that you shouldn't need the typecast, but I'm too jet lagged to think any deeper about it.

You are correct, not sure what I was doing before that I needed to cast it.

…into snack-bar

jelbourn

LGTM aside from one last nit

jelbourn · 2016-10-12T10:42:40Z

src/lib/core/animation/animation.ts

+  static get standardCurve(): string { return 'cubic-bezier(0.4,0.0,0.2,1)'; }
+  static get decelerationCurve(): string { return 'cubic-bezier(0.0,0.0,0.2,1)'; }
+  static get accelerationCurve(): string { return 'cubic-bezier(0.4,0.0,1,1)'; }
+  static get sharpCurve(): string { return 'cubic-bezier(0.4,0.0,0.6,1)'; }


No need for these to be getters; you can just have

static STANDARD_CURVE = 'cubic-bezier(0.4,0.0,0.2,1)';

andrewseguin · 2016-10-31T18:27:29Z

Hey Joey, I don't see that these tests are evaluating the "afterDismissed" subscription. Anything being expected inside these parts of the tests are not being called because the animation done state is not triggering. Were you able to get these working in the tests?

Example of no-op:
snackBarRef.afterDismissed().subscribe(null, null, () => { expect(dismissed).toBeTruthy('Expected the snack bar to be dismissed'); expect(overlayContainerElement.childElementCount) .toBe(0, 'Expected the overlay container element to have no child elements'); });

angular-automatic-lock-bot · 2019-09-06T03:01:26Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

josephperrott added 2 commits September 23, 2016 11:25

feat(md-snack-bar): Create initial MdSnackBar.

7151d93

Merge remote-tracking branch 'upstream/master'

178323c

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Sep 24, 2016

jelbourn reviewed Sep 25, 2016

View reviewed changes

jelbourn added the pr: needs review label Sep 25, 2016

josephperrott force-pushed the snack-bar branch 4 times, most recently from afc2320 to 91a3e3c Compare September 26, 2016 04:05

jelbourn reviewed Sep 26, 2016

View reviewed changes

josephperrott force-pushed the snack-bar branch 3 times, most recently from a27b2fc to 7ec6fd3 Compare September 26, 2016 14:58

Add enter and exit animation to MdSnackBar.

69a0ccf

josephperrott force-pushed the snack-bar branch from 7ec6fd3 to 69a0ccf Compare October 3, 2016 16:37

josephperrott added 3 commits October 3, 2016 09:39

feat(md-snack-bar): Create initial MdSnackBar.

eef6e54

feat(md-snack-bar): Create initial MdSnackBar.

5f3a35f

Add enter and exit animation to MdSnackBar.

37cead1

josephperrott force-pushed the snack-bar branch 2 times, most recently from 4aef5ea to 42febfb Compare October 3, 2016 16:49

Merge branch 'snack-bar' of https://github.com/josephperrott/material2 …

d710b48

…into snack-bar

josephperrott force-pushed the snack-bar branch 2 times, most recently from fa83858 to fbb2d4f Compare October 5, 2016 17:10

jelbourn approved these changes Oct 12, 2016

View reviewed changes

jelbourn added pr: lgtm and removed pr: needs review labels Oct 12, 2016

jelbourn assigned kara Oct 12, 2016

josephperrott force-pushed the snack-bar branch 2 times, most recently from 419a01e to a654207 Compare October 14, 2016 19:10

Merge remote-tracking branch 'upstream/master' into snack-bar

ccc2bdc

josephperrott force-pushed the snack-bar branch from a654207 to ccc2bdc Compare October 14, 2016 22:59

jelbourn merged commit 6df29dc into angular:master Oct 18, 2016

josephperrott deleted the snack-bar branch October 28, 2016 22:00

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019

		import {Subject} from 'rxjs/Subject';


		export class SnackBarState {

feat(snack bar): Add enter and exit animations. #1320

feat(snack bar): Add enter and exit animations. #1320

Uh oh!

Conversation

josephperrott commented Sep 24, 2016

Uh oh!

jelbourn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jelbourn Sep 25, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jelbourn Sep 26, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jelbourn Sep 26, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jelbourn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewseguin commented Oct 31, 2016

Uh oh!

jelbourn Sep 25, 2016 •

edited

Loading

jelbourn Sep 26, 2016 •

edited

Loading

jelbourn Sep 26, 2016 •

edited

Loading